Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add reasons to unsub modal #24109

Merged
merged 13 commits into from
Aug 2, 2024
Merged

feat: add reasons to unsub modal #24109

merged 13 commits into from
Aug 2, 2024

Conversation

zlwaterfield
Copy link
Contributor

@zlwaterfield zlwaterfield commented Jul 31, 2024

Changes

We want to know more about why people are unsubscribing. This will add 5 options, and users can select multiples on top of an open text box. The hope is that more users will fill it out.

Looking for feedback on the UI:
Screenshot 2024-07-31 at 3 52 11 PM

Screenshot 2024-07-31 at 12 01 06 PM

Note: I'm seeing an issue locally where survey events are being triggered a bunch of times; I'm not sure if it's just a local issue but it's unrelated to my PR.

Another note: We've also seen a spike in survey sent/dismissed events this week: https://us.posthog.com/project/2/insights/wDZyeL6N

We will need to update the Slack hook to account for this, I need to figure out where that is configured.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Does this work well for both Cloud and self-hosted?

No

How did you test this code?

Snapshots will get updated

@zlwaterfield zlwaterfield self-assigned this Jul 31, 2024
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

  • chromium: 0 added, 4 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

github-actions bot commented Jul 31, 2024

Size Change: -152 B (-0.01%)

Total Size: 1.07 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.07 MB -152 B (-0.01%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the items of this list just don't look selectable. Checkboxes are probably the best pattern to use in here – they are clearly interactive and clearly multi-select

@zlwaterfield
Copy link
Contributor Author

Hmm, the items of this list just don't look selectable. Checkboxes are probably the best pattern to use in here – they are clearly interactive and clearly multi-select

Cool I can make that change.

@zlwaterfield
Copy link
Contributor Author

Screenshot 2024-08-01 at 5 13 41 PM Screenshot 2024-08-01 at 5 13 56 PM

@zlwaterfield zlwaterfield requested a review from Twixes August 1, 2024 21:14
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

5 snapshot changes in total. 0 added, 5 modified, 0 deleted:

  • chromium: 0 added, 5 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Member

@raquelmsmith raquelmsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love it

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@zlwaterfield zlwaterfield enabled auto-merge (squash) August 2, 2024 00:44
Copy link
Member

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few suggestions for increased UI clarity that are nice-to-haves – overall: :shipit:

'Found a better alternative',
'Poor customer support',
'Too difficult to use',
'Not enough hedgehogs',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL

</Link>
</p>
</LemonBanner>
<p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some extra-large margin here due to p's implicit margin-bottom:

Suggested change
<p>
<p className="mb-0">

(this is important so that the modal doesn't require scrolling)

@zlwaterfield zlwaterfield merged commit 76484cb into master Aug 2, 2024
93 checks passed
@zlwaterfield zlwaterfield deleted the zach/downgrade-reasons branch August 2, 2024 14:26
silentninja pushed a commit to silentninja/posthog that referenced this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants